Spaces - Client NP Migration, Phase 1 by legrego · Pull Request #40856 · elastic/kibana

legrego · 2019-07-11T12:34:01Z

Summary

This introduces a shim in the client-side spaces plugin to start migrating to the new platform.

This PR uses the following NP core/plugin services:

Chrome Nav Control
Feature Catalogue Registry

This PR also performs some ancillary cleanup/prep work for when the rest of the plugin is able to migrate:

Introduce NP "Plugin" in legacy world
Migrate Chrome Nav Control to NP
Remove SpacesNavState, an unnecessary angular service
Update SpacesManager to use NP dependencies (http and notifications)
Initialize SpacesManager once within NP, consume single instance everywhere
Remove almost all injected variables, replace with private API calls
Replace usages of ui/capabilities with the NP equivalent
Migrate Feature Catalogue Registration to NP

Resolves #46255

Out of scope

Management screens - ⛔️ blocked: relies on legacy Kibana plugin/uiExports
Space selector screen door - ⛔️ blocked: relies on legacy Kibana for rendering hidden apps

legrego · 2019-07-11T17:11:22Z

        spacesEnabled,
      };

-      if (spacesEnabled) {


stray code, was not being used in TememetryForm component

elasticmachine · 2019-07-11T19:50:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-11T21:19:24Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-07-11T21:34:40Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

legrego · 2019-07-12T14:50:16Z

 import { SpaceCards } from '../components/space_cards';

 interface Props {
-  spaces?: Space[];


If this.props.spaces was provided, it was via injected variables, which have been removed as part of this PR. Now, the selector screen must request the list of spaces on mount.

legrego · 2019-07-12T14:52:24Z

    );
  };
+
+  private rendePlaceholderMenuItem = (key: string | number): JSX.Element => {


Not strictly required as part of this migration, but it makes for a better user experience when loading the list of available spaces. We often take the near-immediate responses for granted when running locally, and forget that these things can take a noticeable amount of time when hosted in the real world.

++, it actually looks very nice (testing with throttled network connection) 👍

Suggested change

private rendePlaceholderMenuItem = (key: string | number): JSX.Element => {

private rendePlaceholderMenuItem = (key: string | number) => {

elasticmachine · 2019-07-12T14:55:50Z

Pinging @elastic/kibana-security

elasticmachine · 2019-07-12T14:59:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

legrego · 2019-07-12T15:10:17Z

retest

legrego · 2019-12-10T19:49:38Z

@afgomez thanks so much for testing and leaving feedback!

I updated the hook locally to return string | undefined, but the fallout from that cascades throughout the app, to the point where I'm not comfortable making the changes on my own (or as part of this PR)

I'll give this some more thought, but I'm learning towards re-introducing the activeSpace injected var to the legacy platform so that you can continue to consume this as-is, but then your migration to the NP will potentially require you to handle the space id arriving in an async manner.

afgomez · 2019-12-11T09:14:18Z

your migration to the NP will potentially require you to handle the space id arriving in an async manner.

@legrego that should be OK. We will take it into account :)

Thanks!

…-control

legrego · 2019-12-11T14:25:28Z

@afgomez I restored the original functionality, and the log analysis UI appears to function correctly for me now. Would you mind double-checking for me when you get a chance?

afgomez

@legrego thanks for this! I tried again and it works like a charm.

Infra changes LGTM

legrego · 2019-12-13T17:07:35Z

@elasticmachine merge upstream

…o np/spaces-nav-control

…-control

azasypkin

Looks great, thanks for doing this! I've played with a number of basic Spaces related operations - everything worked as expected.

legrego · 2019-12-16T11:29:50Z

@elasticmachine merge upstream

elasticmachine · 2019-12-16T12:46:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: cb44020

History

💔 Build #15512 failed 884e545
💚 Build #15505 succeeded 7cfdcb1
💚 Build #15469 succeeded 5cbb6d9
💚 Build #14790 succeeded 71865a9
💚 Build #14754 succeeded 03f1687

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Spaces - Client NP Migration, Phase 1 (#40856) * shimming NP for spaces client-side plugin * refresh active space in nav control when updated * fix advanced settings screen * allow npStart from unauthed routes * use NP for deriving space management url * remove security's usage of SpacesManager * remove usages of ui/capabilities * fix tests * implement NP plugin interface * remove hack in favor of convention in migration guide * shim feature catalogue registration * streamline nav control, and handle async loading more gracefully * adding opaqueId * fixes from merge * fix merge from master * fixing merge from master * move _active_space route to NP * moving to the NP feature catalogue registry * moving setup to setup phase * optimizing active space retrieval * reverting test isolation change * Apply suggestions from code review Co-Authored-By: Aleh Zasypkin <aleh.zasypkin@gmail.com> * removing unnecessary PluginInitializerContext * updating advanced settings subtitle * using NP anonymousPaths service * additional nav_control_popover cleanup * additional cleanup * testing out onActiveSpaceChange$ property * make the linter happy * make the type checker happy * fixing types * fix merge from master * spaces LP init should run on all pages, not just the kibana app * address nits * fix infra/logs, and the spaces disabled scenario * fix typescript errors * revert changes to infra plugin * reintroducing activeSpace injected var for legacy plugins * fixing react deprecation warning and unhandled promise rejection * restore activeSpace default var * spaces does not need to check its own enabled status * fix from merge Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> * fix backport merge

This reverts commit fcdaed9.

* shimming NP for spaces client-side plugin * refresh active space in nav control when updated * fix advanced settings screen * allow npStart from unauthed routes * use NP for deriving space management url * remove security's usage of SpacesManager * remove usages of ui/capabilities * fix tests * implement NP plugin interface * remove hack in favor of convention in migration guide * shim feature catalogue registration * streamline nav control, and handle async loading more gracefully * adding opaqueId * fixes from merge * fix merge from master * fixing merge from master * move _active_space route to NP * moving to the NP feature catalogue registry * moving setup to setup phase * optimizing active space retrieval * reverting test isolation change * Apply suggestions from code review Co-Authored-By: Aleh Zasypkin <aleh.zasypkin@gmail.com> * removing unnecessary PluginInitializerContext * updating advanced settings subtitle * using NP anonymousPaths service * additional nav_control_popover cleanup * additional cleanup * testing out onActiveSpaceChange$ property * make the linter happy * make the type checker happy * fixing types * fix merge from master * spaces LP init should run on all pages, not just the kibana app * address nits * fix infra/logs, and the spaces disabled scenario * fix typescript errors * revert changes to infra plugin * reintroducing activeSpace injected var for legacy plugins * fixing react deprecation warning and unhandled promise rejection * restore activeSpace default var * spaces does not need to check its own enabled status * fix from merge Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

legrego added 2 commits July 11, 2019 08:30

shimming NP for spaces client-side plugin

f80f888

refresh active space in nav control when updated

452d592

This comment has been minimized.

Sign in to view

fix advanced settings screen

b41073c

This comment has been minimized.

Sign in to view

allow npStart from unauthed routes

1677dc4

legrego commented Jul 11, 2019

View reviewed changes

legrego added 3 commits July 11, 2019 13:17

use NP for deriving space management url

a014d72

remove security's usage of SpacesManager

b0bf4dd

remove usages of ui/capabilities

c46f283

This comment has been minimized.

Sign in to view

legrego added 2 commits July 11, 2019 14:49

fix tests

aed3888

implement NP plugin interface

2aa3b71

legrego added 2 commits July 11, 2019 16:17

remove hack in favor of convention in migration guide

b039a99

shim feature catalogue registration

8c161f2

streamline nav control, and handle async loading more gracefully

3a010f1

legrego commented Jul 12, 2019

View reviewed changes

Comment thread x-pack/legacy/plugins/spaces/server/routes/api/v1/spaces.ts Outdated

legrego commented Jul 12, 2019

View reviewed changes

legrego added Feature:New Platform Feature:Security/Spaces Platform Security - Spaces feature release_note:skip Skip the PR/issue when compiling release notes Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// labels Jul 12, 2019

legrego added 2 commits December 10, 2019 16:14

revert changes to infra plugin

d771522

reintroducing activeSpace injected var for legacy plugins

0186d5f

legrego added 4 commits December 11, 2019 07:41

fixing react deprecation warning and unhandled promise rejection

5b2c16a

Merge branch 'master' of github.com:elastic/kibana into np/spaces-nav…

03f1687

…-control

restore activeSpace default var

e29437b

Merge branch 'master' of github.com:elastic/kibana into np/spaces-nav…

71865a9

…-control

afgomez approved these changes Dec 12, 2019

View reviewed changes

legrego requested a review from azasypkin December 12, 2019 19:56

elasticmachine and others added 6 commits December 13, 2019 09:11

Merge branch 'master' into np/spaces-nav-control

5cbb6d9

spaces does not need to check its own enabled status

2ff6cb8

Merge branch 'np/spaces-nav-control' of github.com:legrego/kibana int…

7cfdcb1

…o np/spaces-nav-control

Merge branch 'master' of github.com:elastic/kibana into np/spaces-nav…

bbfc0bd

…-control

fix from merge

ffb8a7b

Merge branch 'master' of github.com:elastic/kibana into np/spaces-nav…

884e545

…-control

azasypkin approved these changes Dec 13, 2019

View reviewed changes

Merge branch 'master' into np/spaces-nav-control

cb44020

legrego added the v7.6.0 label Dec 16, 2019

legrego merged commit fcdaed9 into elastic:master Dec 16, 2019

legrego deleted the np/spaces-nav-control branch December 16, 2019 13:31

legrego mentioned this pull request Dec 16, 2019

[7.x] Spaces - Client NP Migration, Phase 1 (#40856) #53113

Merged

brianseeders added a commit to brianseeders/kibana that referenced this pull request Dec 17, 2019

Revert "Spaces - Client NP Migration, Phase 1 (elastic#40856)"

40e40f0

This reverts commit fcdaed9.

	private rendePlaceholderMenuItem = (key: string \| number): JSX.Element => {
	private rendePlaceholderMenuItem = (key: string \| number) => {

Conversation

legrego commented Jul 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Out of scope

Uh oh!

This comment has been minimized.

This comment has been minimized.

legrego Jul 11, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

elasticmachine commented Jul 11, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Jul 11, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Jul 11, 2019

💚 Build Succeeded

Uh oh!

Uh oh!

legrego Jul 12, 2019

Choose a reason for hiding this comment

Uh oh!

legrego Jul 12, 2019

Choose a reason for hiding this comment

Uh oh!

azasypkin Oct 30, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 12, 2019

Uh oh!

elasticmachine commented Jul 12, 2019

💔 Build Failed

Uh oh!

legrego commented Jul 12, 2019

Uh oh!

legrego commented Dec 10, 2019

Uh oh!

afgomez commented Dec 11, 2019

Uh oh!

legrego commented Dec 11, 2019

Uh oh!

afgomez left a comment

Choose a reason for hiding this comment

Uh oh!

legrego commented Dec 13, 2019

Uh oh!

azasypkin left a comment

Choose a reason for hiding this comment

Uh oh!

legrego commented Dec 16, 2019

Uh oh!

elasticmachine commented Dec 16, 2019

💚 Build Succeeded

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

legrego commented Jul 11, 2019 •

edited

Loading